Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lnd: label the tx to make it easier to audit #282

Merged
merged 3 commits into from
Feb 19, 2024

Conversation

YusukeShimizu
Copy link
Contributor

@YusukeShimizu YusukeShimizu commented Feb 11, 2024

Ensure that all lnd transactions related to peerswaps are identifiable by the label peerswap.
This makes it easier to audit the transactions from faraday.
This is performed by LND's LabelTransaction RPC.

If it fails, it is logged and subsequent processing is continued.

#254.

Samples are shown below.

  • peerswap -- Opening(swap id=b171ee)
  • peerswap -- ClaimByCoop(swap id=b171ee)
  • peerswap -- ClaimByCsv(swap id=b171ee)
  • peerswap -- ClaimByInvoice(swap id=b171ee)

@YusukeShimizu YusukeShimizu force-pushed the label-transaction branch 4 times, most recently from 197c802 to b07c2b3 Compare February 11, 2024 07:40
@YusukeShimizu YusukeShimizu marked this pull request as ready for review February 11, 2024 07:46
Ensure that all lnd transactions related to peerswaps are identifiable
by the label `peerswap`.
This makes it easier to audit the transactions from faraday.
This is performed by LND's LabelTransaction RPC.
Since CLN has no such function, no label is assigned.
Since label recording is not an essential requirement for swap,
if it fails, it is logged and subsequent processing is continued.
Comment on lines +220 to +223
&walletrpc.LabelTransactionRequest{
Txid: txIDHash.CloneBytes(),
Label: label,
Overwrite: true})
Copy link

@mrfelton mrfelton Feb 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should include a reference to the swap that the transaction relates to in the label. e.g. the swap id, or first few characters of swap id (this is what Lightning Loop does, I think just to keep the labels shorter and more readable)

Here are some examples of the onchain transaction labels added by Loop:

loopd -- OutSweepSuccess(swap=40f86b)
loopd -- InHtlc(swap=e5145f)
loopd -- InSweepTimeout(swap=3cc561)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With reference to loopd, the following labels are set.

  • peerswap -- Opening(swap id=b171ee)
  • peerswap -- ClaimByCoop(swap id=b171ee)
  • peerswap -- ClaimByCsv(swap id=b171ee)
  • peerswap -- ClaimByInvoice(swap id=b171ee)

@YusukeShimizu YusukeShimizu mentioned this pull request Feb 13, 2024
Add labels package to handle labeling on-chain transactions.
* Opening label
* claim by CSV label
* claim by cooperative close label
@mrfelton
Copy link

mrfelton commented Feb 15, 2024

I initiated a swapout and the unconfirmed onchain transaction got it's label applied.

This is the transaction from the node initiating the swapout (lnd1):

{
            "tx_hash":  "bf66241fd8c55aa7024b762289660d10ad1bc615f60132e2cd62c43c712db4ec",
            "amount":  "992000",
            "num_confirmations":  0,
            "block_hash":  "",
            "block_height":  0,
            "time_stamp":  "1707997184",
            "total_fees":  "0",
            "dest_addresses":  [
                "bcrt1qll65rvr3ggvzz50sen6xvw99spmrztgywvzzau"
            ],
            "output_details":  [
                {
                    "output_type":  "SCRIPT_TYPE_WITNESS_V0_PUBKEY_HASH",
                    "address":  "bcrt1qll65rvr3ggvzz50sen6xvw99spmrztgywvzzau",
                    "pk_script":  "0014fff541b07142182151f0ccf46638a58076312d04",
                    "output_index":  "0",
                    "amount":  "992000",
                    "is_our_address":  true
                }
            ],
            "raw_tx_hex":  "02000000000101dad897bb530ecc0fe26fe6aacab4845ffd64a02479671432d19dd4493c28a5a50000000000000000000100230f0000000000160014fff541b07142182151f0ccf46638a58076312d04054730440220250226405fae6d453324b16877a4c7ad37c06248342f0e588238848ee84d0d0f02201373210c9803a1332c78f2db15ae71484c4d755300cfc93df93d5fc8e09ab73e01208a3a831e09bebeb970701575505e13db0d5caf924460b7ee3e26acd1f7c911ca0000992103e5389980897a881f233fcde840a9d370a04441d8fa5767fcc98c959e64840947ac642103e5389980897a881f233fcde840a9d370a04441d8fa5767fcc98c959e64840947ac6482012088a820ed18813b1a45fd636987d733faacc1f323be169fbff8e75b1db5b478d7b6a83588682103162589014005b0ecb17200b027687d07b12f94e1f2ef5d56b37df68dc6888060ac6702f003b26800000000",
            "label":  "peerswap -- ClaimByInvoice(swap id=ecaf24)",
            "previous_outpoints":  [
                {
                    "outpoint":  "a5a5283c49d49dd13214677924a064fd5f84b4caaae66fe20fcc0e53bb97d8da:0",
                    "is_our_output":  false
                }
            ]
        }

And on the other node (lnd2):

{
            "tx_hash":  "a5a5283c49d49dd13214677924a064fd5f84b4caaae66fe20fcc0e53bb97d8da",
            "amount":  "-1007650",
            "num_confirmations":  6,
            "block_hash":  "0c859654bb9a9c3413fcef8cf9b11fc442d697550c216b5f6fe6eabe77518df2",
            "block_height":  116,
            "time_stamp":  "1707997172",
            "total_fees":  "7650",
            "dest_addresses":  [
                "bcrt1quyukzat3zrzuqhvt2accxkkt6ccz0u8j74h0azxqh5yur59yqhfsqd7ygs",
                "bcrt1q0eh929jvkxazzajttpn3x47zt8wqzyratckfek"
            ],
            "output_details":  [
                {
                    "output_type":  "SCRIPT_TYPE_WITNESS_V0_SCRIPT_HASH",
                    "address":  "bcrt1quyukzat3zrzuqhvt2accxkkt6ccz0u8j74h0azxqh5yur59yqhfsqd7ygs",
                    "pk_script":  "0020e13961757110c5c05d8b5771835acbd63027f0f2f56efe88c0bd09c1d0a405d3",
                    "output_index":  "0",
                    "amount":  "1000000",
                    "is_our_address":  false
                },
                {
                    "output_type":  "SCRIPT_TYPE_WITNESS_V0_PUBKEY_HASH",
                    "address":  "bcrt1q0eh929jvkxazzajttpn3x47zt8wqzyratckfek",
                    "pk_script":  "00147e6e55164cb1ba21764b58671357c259dc01107d",
                    "output_index":  "1",
                    "amount":  "98992350",
                    "is_our_address":  true
                }
            ],
            "raw_tx_hex":  "02000000000101e4ccaec69876d955e9f208ffb7d82e8c4bfe1b67da654587290641495ae2103f0200000000ffffffff0240420f0000000000220020e13961757110c5c05d8b5771835acbd63027f0f2f56efe88c0bd09c1d0a405d3de80e605000000001600147e6e55164cb1ba21764b58671357c259dc01107d0247304402205612f6d8d8c4a6a5cca015cee93b52b3714d43cec99ec78616cc6d58c8d84d1b02201bfcacc440cb0b0e35d0d606b8fd1ae356f2c45d46807cab3e89bf388835abd5012102747f4287846b4bf545cc91282c6b69dd4ca7c37a4e5080cb48db37451e14046500000000",
            "label":  "peerswap -- Opening(swap id=ecaf24)",
            "previous_outpoints":  [
                {
                    "outpoint":  "3f10e25a49410629874565da671bfe4b8c2ed8b7ff08f2e955d97698c6aecce4:2",
                    "is_our_output":  true
                }
            ]
        }

However I also get these following errors in the logs for the initiating node (lnd1)

lnd1-1       | 2024-02-15 11:39:32.507 [ERR] RPCS: [/walletrpc.WalletKit/LabelTransaction]: cannot label transaction not known to wallet
peerswap1-1  | 2024/02/15 11:39:32 [DEBUG] [grpc_conn]: grpc_retry attempt: 0, got err: rpc error: code = Unknown desc = cannot label transaction not known to wallet
peerswap1-1  | 2024/02/15 11:39:32 [INFO] Error labeling transaction. txid: a5a5283c49d49dd13214677924a064fd5f84b4caaae66fe20fcc0e53bb97d8da, label: peerswap -- Opening(swap id=ecaf24), error: rpc error: code = Unknown desc = cannot label transaction not known to wallet

a5a5283c49d49dd13214677924a064fd5f84b4caaae66fe20fcc0e53bb97d8da refers to the previous output from the incoming transaction, which is a transaction that is only tracked by the other node. I don't think the receiving node should be attempting to apply a label to this transaction that isn't tracked by the receiving node.

Receiving nodes should not attempt to apply a label to this transaction
that is not being tracked by the receiving node
@YusukeShimizu
Copy link
Contributor Author

Receiving node label operation removed.
2ce98c1

@mrfelton
Copy link

Tested as working on lnd.

@nepet
Copy link
Contributor

nepet commented Feb 19, 2024

Looks good to me, thanks @YusukeShimizu!
Label for core-lightning, bitcoind and elementsd can be implemented once there is functionality to do so.

ACK 2ce98c1

@nepet nepet merged commit 0c4e692 into ElementsProject:master Feb 19, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants